Adds multiple simultaneous slicers support #3758

jellybean2004 · 2025-11-14T13:34:17Z

Description

Implemented support for multiple simultaneous slicers on 2D plots, allowing users to create, manage, and switch between different slicer instances.

A unique ID is assigned to each slicer instance so they can be viewed simultaneously on the 2D fitting plot, and each generates an independent plot.
A list of all active slicers is added in the Slicer Parameters window so users can select and tweak them individually. The selected slicer gets automatically updated when a slicer is added or modified in the 2D plot. Users can also delete individual slicers from there.
Slicers have different colours to help differentiate them.

How Has This Been Tested?

Functionality tested in the program, works as expected.

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

There is nothing that needs documenting
Documentation changes are in this PR
There is an issue open for the documentation (link?)

Installers

There is a chance this will affect the installers, if so
- Windows installer (GH artifact) has been tested (installed and worked)
- MacOSX installer (GH artifact) has been tested (installed and worked)
- Wheels installer (GH artifact) has been tested (installed and worked)

Licensing (untick if necessary)

The introduced changes comply with SasView license (BSD 3-Clause)

krzywon · 2025-11-14T21:59:38Z

I'm testing this locally and think this is looking incredibly promising! I can easily add any number of slicers to a 2D plot.

Questions and notes I have:

Should multiple slicers be added together, treated separately, or should there be a user option to switch? In the current implementation, they are all treated separately.
Is it possible to add a list of active slicers to the context menu, similar to how 1D plots list individual plots? This would help enable features (including ones I'm suggesting below), like removing a single slicer, changing the color of the slicer, etc.
Is it possible to remove only one of the slicers? This would allow a user to compare two slices, and only keep the one they want without having to keep another slicer visible.
Previous versions replaced the existing slicer when a new slicer was selected. This changes that paradigm. Is this desired, or should we include a way for the user to either add or replace a slicer?
Selecting a slicer on the plots highlights that slicer in the slicer parameters window, which is a nice feature!
Multiple, overlapping slicers are difficult to differentiate. Is it possible to make them all different colors or even give the user an option for colors?

jellybean2004 · 2025-11-15T10:01:46Z

I'm testing this locally and think this is looking incredibly promising! I can easily add any number of slicers to a 2D plot.

Questions and notes I have:

Should multiple slicers be added together, treated separately, or should there be a user option to switch? In the current implementation, they are all treated separately.

Is it possible to add a list of active slicers to the context menu, similar to how 1D plots list individual plots? This would help enable features (including ones I'm suggesting below), like removing a single slicer, changing the color of the slicer, etc.

Is it possible to remove only one of the slicers? This would allow a user to compare two slices, and only keep the one they want without having to keep another slicer visible.

Previous versions replaced the existing slicer when a new slicer was selected. This changes that paradigm. Is this desired, or should we include a way for the user to either add or replace a slicer?

Selecting a slicer on the plots highlights that slicer in the slicer parameters window, which is a nice feature!

Multiple, overlapping slicers are difficult to differentiate. Is it possible to make them all different colors or even give the user an option for colors?

Hi @krzywon! Welcome back, and thank you for your comments.

So this is the first step in the whole project I have at hand, which is just to enable the addition of multiple slicers at once, bringing it closer to the features available in Grasp. The next step is to plot them on the same plot. Finally, to link the slicers so that cases like peaks symmetrically distributed around the centre can be handled more easily.

I've just pushed a commit that implements deleting a single slicer.

Regarding the list of active slicers, I have updated the Slicer Parameters window to include a list of all the slicers shown in the plot. There, you can also check a particular slicer to tweak the parameters, and with the new commit, you can delete just the selected slicer too. Is this what you mean?

I could add a checkbox somewhere so users can either have the newly added multi-slicers or keep the previously supported single slicer, if that is something useful to keep.

You did raise a good point about the slicers being the same colour, and therefore being hard to differentiate (I was ignoring that, but I guess I have been caught 😅) I will give a shot at implementing that.

rozyczko

A few minor comments to the source and questions about implementation.

src/sas/qtgui/Plotting/Slicers/BaseInteractor.py

src/sas/qtgui/Plotting/Slicers/AnnulusSlicer.py

src/sas/qtgui/Plotting/Slicers/SectorSlicer.py

src/sas/qtgui/Plotting/SlicerParameters.py

jellybean2004 · 2025-11-16T10:32:23Z

@krzywon, I have changed the slicer colours to be different, so it is easier to differentiate. I have also added the functionality of individual slicer deletion, which can be done from the slicer parameter window.

@rozyczko, I have implemented most of your suggestions. I will now work on refactoring out the ID generation, as it is the same for each slicer.

Please let me know if there is anything else I could add, or something I missed.

butlerpd

Nice work though I have not looked closely at the code yet. This is mainly a functionality review based on window 11 testing of the installer.

Clear slicers does not clear the box sum box on the plot so once a box sum is slicer is activated, the box on the 2D plot is permanent and we have to close the whole plot before another slicer can be used cleanly. Not sure if this predates this PR in which case another issue might be appropriate, but prior to this PR a new slicer removed the old anyway?
There are really irritating white lines that randomly appear (apparently when another window passes over the plot?) but this seem to be there from before this pull request? In which case an issue might be the better place to worry about this.
I like that all slicers, even the annulus (and box sum though IMO we should eventually remove that as a thing) allows for multiple slicers. The color coding helps, but it is a bit unintuitive that the new slicer falls directly on top of the old one. To the newbie it looks like nothing happened? maybe just offset the interactors by a small fraction from the first one each time. If the third is on top of the second it won't matter as by that point it should be clear? and most people will move the second to where they want it before asking for a third slicer?
Alternatively I guess the offset could increment each time? This is more complicated for the annulus which does not have a center that ties the two sides together. Not sure how to deal with that, but I think the colloid orientation people will use the multiple annulus a lot.
Related, the top color is the color for the last slice. However, if one grabs the center point and moves the ROI, it is the very first one that gets moved. This is very counterintuitive. At first I thought that resizing the one I moved was not working as I assumed it should be updating the last 1D plot (the one on top).
Clearing slicers does not clear the 1D plots. Should it? I think so, specially now that we may have several plots from one 2D plot. Also @ChristianWoegerbauer work to promote the slicer plot to a regular data set, if the user wants to maitain that data they presumably will have pressed send to fitting?
It is not clear to me that there is a use case for having two different types of slicers on the plot at once? And if the normal desire is to change slicer types it might be useful to revert that behavior to clearing the slicers and creating the new one. That said, all the user has to learn to do is to clear slicers before asking for a different slicer type so maybe it is OK to leave as is for now?

butlerpd · 2025-11-17T14:42:39Z

Another question: what happens if I add enough slicers to go through the list of 15 defined colors? do the colors recycle back at the beginning?

jellybean2004 · 2025-11-17T14:48:59Z

Another question: what happens if I add enough slicers to go through the list of 15 defined colors? do the colors recycle back at the beginning?

Yes, the colours cycle back, but hopefully no one is adding more than 15 slicers 😅

krzywon

Adding a BoxSum deletes all other slicers, except existing BoxSum slicers, but previous BoxSum slicer interactors are no longer draggable on the plot. Clear Slicers does not remove any BoxSum, including the one the is still interactive.

Otherwise, see code comments.

Thanks for making the individual slicers removable and for the color work!

src/sas/qtgui/Plotting/Plotter2D.py

krzywon · 2025-11-17T14:05:50Z

src/sas/qtgui/Plotting/Plotter2D.py

+        # TODO: generalize this and put it in GuiUtils so that we can use it elsewhere
+        tempPlotsToRemove = []
+        slicer_type_id = 'Slicer' + self.data0.name
+        for itemIndex in range(item.rowCount()):


I'm almost certain this could be simplified into a single loop and a more compact if tree. CodeScene will gate on something like this, which is why I am noting it. I'll try to find a cleaner solution today.

src/sas/qtgui/Plotting/Plotter2D.py

src/sas/qtgui/Plotting/SlicerParameters.py

src/sas/qtgui/Plotting/Slicers/AnnulusSlicer.py

src/sas/qtgui/Plotting/Slicers/SlicerUtils.py

butlerpd

having looked at the code a bit and the other comments I think this is essentially ready to go after fixing the issues named or pushing them to new issues for a future PR. Probably don't want this PR to grow too much?

krzywon · 2025-11-19T14:25:27Z

@butlerpd and @jellybean2004 - The box sum issues I've mentioned are new and should be addressed before I will approve. The one that is required is the box sum not being cleared when slicers are cleared so multiple box sum images end up on the plot. The box sum clearing all other slicers when a new box sum is generated is how it currently works, but is also different than the new functionality, so I'm conflicted on this.

butlerpd · 2025-11-20T22:54:19Z

Using the long flight to do a more thorough functionality review. The following is the result

FIXED

After a 2nd slicer is added, manually moving it now moves the topmost (last) slicer. 👍

PROBLEMS:

If one or more slices are created before launching the Edit Slicer Parameters panel
- The top half of the new editor panel shows the first slice as selected (its radio button is highlighted) but the parameters shown in the bottom box are for the last slice. Normally the bottom box should match the selected slice from the top box and it does except for this startup case.
- Also, the create new slicer dropdown in the top box shows the type selected instead of none and new slices of the same type cannot be created from the panel until a different type of slicer is chosen. Once reset, creating a new slice from the top panel of the Edit Slicer Parameters works and will add a new slice, make its radio button selected and return to showing none in ehe create new drop down box, thus allowing multiple slices of the same type again.
- Choosing none in the create new box when it is not already none (i.e. in this edge case described above since it is otherwise always none) effectively does a "clear slicers" before closing the panel altogether.
I somehow managed at one point (after several different types of slicers and several of each and deleting some 1D plots to see things) to get to a place where clear slicer cleared all but two sector slices. They were apparently ghosts as clear slicers was no longer a choice. Adding another slicer and trying to clear slicers again only cleared the new slicer. This may need more investigation as I could not quickly reproduce.

OTHER THINGS

There is no way to remove all the subplots created either from the data explorer OR by clearing slicers etc. I found that subplots cannot be deleted from the DataExplorer in general. Only the top level data can be deleted. Doing so is the only way to delete all subplots. So this would be a different issue probably unless we think clear slicers should do that? Not sure that would be the desired behavior but thought I'd raise the question.
However, and this may have been discussed previously, I wonder it clearing slicers should not at least remove all the associated plots as part of the effort to keep from cluttering up the window? Since they are now permanent subplots in the DataExplorer (as discussed above) people can still plot them whenever they want? Maybe a question to ask at a fortnightly meeting?
The batch tab only allows picking the available 2D plots. That means that clicking apply will act on ALL the 1D plots associated with the selected 2D plots. There is no way to pick only some 1D plots. Is that a problem? That may be the desired behavior but worth bringing up as a question I figure.
I thought there was a third tab being added? Or am I thinking of something else? or a discussion where the answer was no?

jellybean2004 · 2025-11-24T10:29:03Z

@krzywon @butlerpd Sorry for the delay in getting back at this. I was on annual leave last week, so I just got a chance to get back at it.

I have not fixed the issues with box sum not getting cleared properly. When other slicers are active, and you try to open box sum, it will clear everything to add the box sum. Similarly, if box sum is active and you want to add another slicer, it will clear box sum.

Please let me know once you have a go at it if everything is now working as expected.

jellybean2004 · 2025-11-24T11:00:26Z

@butlerpd Pushed some commits fixing the problems you mentioned. I'll look in to the last problem.

Regarding your last question, yes, a third tab with the 1D slicer plots is being added. That will be a part of another PR, which will be about plotting multiple slicers into one plot.

I am also adding functionality to delete individual 1D plots as a part of this PR. So the question would be whether or not we want to delete the plots as we delete the slicers, or if the user can individually delete plots from this tab and clearing slicers will not affect plots.

I have not looked much into the batch tab, but if that may also be something we can discuss in a fortnightly meeting.

butlerpd · 2025-12-01T21:33:45Z

@jellybean2004 - now my turn to apologize for being out. I will try to review this soon. I would think it should be ready to go - there was really only one reproducible issue. Everything listed under "other" I'm thinking are probably new Issues but thought I'd flag them in case you felt they were part of what you are doing here. Very happy to move those to other issues.

butlerpd · 2025-12-02T00:28:42Z

This all looks good to me now. I was able to throw an error at one point but was never able to reproduce it. I think it is ready to merge from the functionality side. I note Jeff still has comments? Also I think the documentation needs more work. It is not at all clear to me. I suspect it is time to add a screen shot or two and a bit more detail about the context menu vs the slicer parameter window. Actually, that whole plotting data/model section (graph_help) probably needs a facelift.

Given that I would vote for merging this and add a documentation upgrade as a new issue.

…for any such evenys

krzywon

I think this might be ready to merge! One code suggestion that I'll leave to @jellybean2004 to implement or not.

A future improvement I could see: plot all slicers on the same plot (differentiating phi vs Q of course), but this might be best once the plotting refactor is better integrated.

krzywon · 2025-12-02T15:50:14Z

src/sas/qtgui/Plotting/Plotter2D.py

+        self._slicer_color_index += 1
+        return color

+    def _removeSlicerPlots(self):


As I said in my last review, this can likely be simplified to something like (partially tested):

Suggested change

def _removeSlicerPlots(self):

def _recurse_plots_to_remove(self, item):

temp_plots_to_remove = []

slicer_type_id = 'Slicer' + self.data0.name

for item_index in range(item.rowCount()):

child = item.child(item_index)

if child.hasChildren():

self._recurse_plots_to_remove(child)

if not (isinstance(child.data(), (Data1D, Data2D))

and hasattr(child.data(), 'type_id')

and slicer_type_id in child.data().type_id

and child.rowCount() > 0

and child.child(0).text()):

temp_plots_to_remove.append(item.child(item_index))

for plot in temp_plots_to_remove:

item.removeRow(plot.row())

def _removeSlicerPlots(self):

"""

Clear the previous slicer plots

"""

# Clear the old slicer plots so they don't reappear later

if not hasattr(self, '_item'):

return

item = self._item

if self._item.parent() is not None:

item = self._item.parent()

self._recurse_plots_to_remove(item)

krzywon · 2025-12-03T15:18:38Z

src/sas/qtgui/Plotting/SlicerParameters.py

+                                             QtWidgets.QMessageBox.No)
+        if reply == QtWidgets.QMessageBox.No:
+            return


Good use of defaults here! This will catch the window closing by the ESC key, or the window X.

rozyczko reviewed Nov 15, 2025

View reviewed changes

jellybean2004 force-pushed the multi_slicer_feature branch from 66d67ec to ff94b84 Compare November 17, 2025 12:04

butlerpd reviewed Nov 17, 2025

View reviewed changes

krzywon requested changes Nov 17, 2025

View reviewed changes

butlerpd approved these changes Nov 17, 2025

View reviewed changes

jellybean2004 mentioned this pull request Nov 24, 2025

Generalise checking if items are plots in Plotter2D #3788

Open

jellybean2004 force-pushed the multi_slicer_feature branch from 226f03a to 1cd36a6 Compare December 2, 2025 13:20

jellybean2004 added 12 commits December 2, 2025 13:31

Reformats slicer files

4f10704

Enables multiple slicers on the same plot simultaneously

b896d0f

Creates a new plot for a new slicer of the same type

115320e

Fixes bug not properly removing slicers, and adds exception handling …

878faa6

…for any such evenys

Patches slicer issues

d68944e

Adds slicer list and selection in params window

a8c8641

Auto selection of current slider in slicers list

08bc43d

Updates docs

19f62ff

Implements single slicer delete button in slicer param window

ec36601

Differentiate slicers with (colour-blind friendly!) colours

2dba531

Fixes create new slicer button

e7f4920

Enables replacement of slicers

f7bc328

jellybean2004 added 17 commits December 2, 2025 13:32

Changes slicer list to radio buttons

4db7c03

Removes unrequried try excepts

5236198

Moves Delete button in Slicer Params to Slicers list

e959e98

Deletion bug fix

d09bd78

Proper implementation of individual deletion

444c766

Check implementation from comments

70e0584

Patch for box deletion issues

96b60c3

Refactor out the unique plot ID generation for slicers

b140d6e

Combobox goes back to None after slicer is created from SP window

2d3a3e7

Updates docs

1f1c0df

Proper box sum handling

0aadcbc

Proper handling of slicer removal

b2ec63a

Makes BoxSum and other slicer mutually exclusive

02def69

Fixes slicer param startup not checking correct RB

dc58ce3

None does not clear slicers

f79c968

Removes commented code

6fc65b5

Recursive checking for ID generation

24b2c93

jellybean2004 force-pushed the multi_slicer_feature branch from 1cd36a6 to 24b2c93 Compare December 2, 2025 13:32

krzywon approved these changes Dec 3, 2025

View reviewed changes

-    def _removeSlicerPlots(self):
+       def _recurse_plots_to_remove(self, item):
+        temp_plots_to_remove = []
+        slicer_type_id = 'Slicer' + self.data0.name
+        for item_index in range(item.rowCount()):
+            child = item.child(item_index)
+            if child.hasChildren():
+                self._recurse_plots_to_remove(child)
+            if not (isinstance(child.data(), (Data1D, Data2D))
+                    and hasattr(child.data(), 'type_id')
+                    and slicer_type_id in child.data().type_id
+                    and child.rowCount() > 0
+                    and child.child(0).text()):
+                        temp_plots_to_remove.append(item.child(item_index))
+        for plot in temp_plots_to_remove:
+            item.removeRow(plot.row())
+    def _removeSlicerPlots(self):
+        """
+        Clear the previous slicer plots
+        """
+        # Clear the old slicer plots so they don't reappear later
+        if not hasattr(self, '_item'):
+            return
+        item = self._item
+        if self._item.parent() is not None:
+            item = self._item.parent()
+        self._recurse_plots_to_remove(item)

Adds multiple simultaneous slicers support #3758

Are you sure you want to change the base?

Adds multiple simultaneous slicers support #3758

Conversation

jellybean2004 commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Review Checklist:

Uh oh!

krzywon commented Nov 14, 2025

Uh oh!

jellybean2004 commented Nov 15, 2025

Uh oh!

rozyczko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jellybean2004 commented Nov 16, 2025

Uh oh!

butlerpd left a comment

Choose a reason for hiding this comment

Uh oh!

butlerpd commented Nov 17, 2025

Uh oh!

jellybean2004 commented Nov 17, 2025

Uh oh!

krzywon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krzywon Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

butlerpd left a comment

Choose a reason for hiding this comment

Uh oh!

krzywon commented Nov 19, 2025

Uh oh!

butlerpd commented Nov 20, 2025

Uh oh!

jellybean2004 commented Nov 24, 2025

Uh oh!

jellybean2004 commented Nov 24, 2025

Uh oh!

butlerpd commented Dec 1, 2025

Uh oh!

butlerpd commented Dec 2, 2025

Uh oh!

krzywon left a comment

Choose a reason for hiding this comment

Uh oh!

krzywon Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

krzywon Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jellybean2004 commented Nov 14, 2025 •

edited

Loading